Repository navigation
Conversation
HtmlRewriter parses with character reference conversion disabled so that references are re-emitted verbatim in the rewritten content. A consequence is that the text of a <title> reaches the parser in several pieces, of which only the first was kept: "Tom & Jerry" gave "Tom" and "L&openzim#39;Illustre Maurin" gave "L". warc2zim uses this value as the ZIM entry title, so any title with an ampersand or an escaped apostrophe came out truncated in title search and suggestions. Title pieces are now collected, with references decoded, and joined when the element closes. Rewritten content is untouched, and the first title which has text still wins: a document with an empty title in head keeps using the title of a later inline SVG. A document whose title element was never closed now gets its whole title instead of the first piece.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #402 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 42 42
Lines 2947 2961 +14
Branches 452 454 +2
=========================================
+ Hits 2947 2961 +14 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@pawank925 I appreciate you are working hard to fix things, but opening a PR on an issue which has been reported by someone which is only waiting for confirmation on how we want to implement the fix, did the hard work of reporting the issue cleanly, analyzing it and already proposed to push a PR is rude. It is the second time I warn you about this is not the way opensource contributions works (at least at openZIM / Kiwix). I will now have to be more strict with you: please do not push a PR (and ideally do not work on an issue, but this is your call) before the issue is assigned to you. I will be very strict with this rule from now and close any PR which does not follow this rule. |
HtmlRewriterfeedsHTMLParserwithconvert_charrefs=False, so that entity and character references are re-emitted verbatim in the rewritten content. That hands the text of a<title>over in several calls, andhandle_datakept only the first piece:<title>Tom & JerryTomTom & JerryRock & Roll – My BlogRockRock & Roll – My BlogL'Illustre MaurinLL'Illustre Maurinwarc2zim uses this value as the ZIM entry title (
Rewriter.rewrite_html()), so a title with an ampersand or an escaped apostrophe ends up truncated in title search and suggestions.The pieces are now collected as they arrive, with references decoded, and joined when the title element closes. Nothing is added or changed in the output stream, so
contentis exactly what it was; only the extracted title changes.This keeps both behaviours the current implementation has, which is why I went for it rather than the two BeautifulSoup variants discussed in the issue (a second parse of the document costs ~15% on the rewrite, per the measurements in #401):
<title>which has text still wins: with an empty<title></title>in head, the title of a later inline SVG is used;html_rewrite_contextis per-tag and I did not touch that.One case changes beyond the bug itself: a document whose title element is never closed (
<title>Unclosed & title) yielded its first piece, and now yields its whole text, so_save_title()also runs atclose().Tests cover named (
é), decimal (&,'), hex (') and bare-&references, whitespace stripping, a second title in body, an empty head title followed by an inline SVG, an unclosed title, and that the references survive in the rewritten content.ruff check/ruff formatandpyright(strict) are clean, andtests/rewritingpasses (635).Fixes #401